Move sObject Refresh command to apex module and improve telemetry #1236

brpowell · 2019-04-05T20:48:02Z

What does this PR do?

The command code has been moved to the apex module, and it now kicks off the process once the language server has finished indexing the classes. This is to minimize the language server initialization clashing with pulling down all of the classes. The setting to enable or disable refresh on start is still present, but it is now under the apex configuration. Send additional telemetry data about the refresh process.

What issues does this PR fix or reference?

W-6017035

…into bp/refreshDefault

codecov · 2019-04-05T21:20:04Z

Codecov Report

Merging #1236 into develop will increase coverage by 0.04%.
The diff coverage is 89.65%.

@@             Coverage Diff             @@
##           develop    #1236      +/-   ##
===========================================
+ Coverage    70.93%   70.98%   +0.04%     
===========================================
  Files          198      198              
  Lines         7469     7471       +2     
  Branches       787      786       -1     
===========================================
+ Hits          5298     5303       +5     
+ Misses        2013     2010       -3     
  Partials       158      158

Impacted Files	Coverage Δ
...rcedx-vscode-core/src/settings/sfdxCoreSettings.ts	`60% <ø> (+3.75%)`	⬆️
...code-apex/src/commands/forceGenerateFauxClasses.ts	`88.33% <100%> (ø)`
...faux-generator/src/generator/fauxClassGenerator.ts	`90.67% <82.35%> (-0.07%)`	⬇️
...forcedx-vscode-core/src/channels/channelService.ts	`83.33% <0%> (-2.39%)`	⬇️
...edx-vscode-core/src/telemetry/telemetryReporter.ts	`53.52% <0%> (+2.81%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9049c0...00a53fd. Read the comment docs.

ruthemmanuelle · 2019-04-05T21:21:42Z

packages/salesforcedx-vscode-apex/src/messages/i18n.ts

-  colorizer_statusbar_hover_text: 'Highlight Apex Code Coverage'
+  colorizer_statusbar_hover_text: 'Highlight Apex Code Coverage',
+  force_sobjects_no_refresh_if_already_active_error_text:
+    'A refresh of your sObject definitions is already underway. If you need to restart the process, cancel the running task.'


packages/salesforcedx-vscode-core/src/index.ts

praksb · 2019-04-08T18:33:50Z

@brpowell Is it possible to make the change to fetch the files before we start indexing in LSP? Otherwise this change would be a potential source of performance issue since I think an sobject fetch would cause us to reinitialize again. So we would have to run index once at startup and again on an sobject fetch.

brpowell · 2019-04-08T19:33:11Z

@brpowell Is it possible to make the change to fetch the files before we start indexing in LSP? Otherwise this change would be a potential source of performance issue since I think an sobject fetch would cause us to reinitialize again. So we would have to run index once at startup and again on an sobject fetch.

You mean wait for sObject refresh to be done and then initialize the LSP? Or kick it off in parallel? If it's the former I'd say that might be a burden for the user to have to wait for refresh to finish to start using lsp features.

lcampos · 2019-04-08T19:37:24Z

@praksb @brpowell indexing and recompilations are good things to look out for with these changes. Keep in mind that we don't know how long the fetching of sobjects will take since that'll depend on the org's shape. You can look at existing telemetry to get an idea, but it could be that indexing local code finishes before the sobject fetch call.

praksb · 2019-04-08T20:07:27Z

@brpowell I was thinking of the former (wait for sobject refresh to be done not run in parallel). But, I do see your point that the refresh might take a while. We can check this in as is and wait for customer feedback. The possible perf impact should also be minimal since we do a refresh only once now and that too only when the folder didn't exist.

@brpowell Is it possible to make the change to fetch the files before we start indexing in LSP? Otherwise this change would be a potential source of performance issue since I think an sobject fetch would cause us to reinitialize again. So we would have to run index once at startup and again on an sobject fetch.

You mean wait for sObject refresh to be done and then initialize the LSP? Or kick it off in parallel? If it's the former I'd say that might be a burden for the user to have to wait for refresh to finish to start using lsp features.

packages/salesforcedx-vscode-apex/package.json

brpowell · 2019-04-09T23:48:47Z

packages/salesforcedx-sobjects-faux-generator/src/generator/fauxClassGenerator.ts

@@ -24,6 +24,21 @@ export interface CancellationToken {
  isCancellationRequested: boolean;


Changes in this file are for improving telemetry data around this command

brpowell · 2019-04-09T23:49:55Z

packages/salesforcedx-vscode-apex/package.json

+          "type": [
+            "boolean"
+          ],
+          "default": false,


As we discussed this morning, the default behavior will now be off. The PR is mostly focused on moving the code to the apex extension and improving telemetry data now.

brpowell · 2019-04-09T23:50:50Z

packages/salesforcedx-vscode-apex/src/commands/forceGenerateFauxClasses.ts

+      console.log('Generate success ' + result.data);
+      this.logMetric(commandName, startTime, result.data);
+    } catch (result) {
+      console.log('Generate error ' + result.error);


Now log errors and successes with the number of objects fetched - if the command got far enough to collect that information.

brpowell · 2019-04-09T23:51:31Z

packages/salesforcedx-vscode-apex/src/index.ts

@@ -59,14 +67,10 @@ export async function activate(context: vscode.ExtensionContext) {
  }

  // Telemetry
-  if (sfdxCoreExtension && sfdxCoreExtension.exports) {
-    sfdxCoreExtension.exports.telemetryService.showTelemetryMessage();


@lcampos removed the telemetry message based on our discussion

lcampos · 2019-04-10T00:15:44Z

packages/salesforcedx-vscode-apex/package.json

@@ -205,6 +206,10 @@
      {
        "command": "sfdx.force.apex.test.last.method.run",
        "title": "%force_apex_test_last_method_run_text%"
+      },
+      {
+        "command": "sfdx.force.internal.refreshsobjects",


You are missing porting the command under commandPalette.

packages/salesforcedx-vscode-apex/src/index.ts

lcampos · 2019-04-10T00:26:25Z

packages/salesforcedx-vscode-apex/src/telemetry/telemetry.ts

@@ -79,6 +87,29 @@ export class TelemetryService {
    }
  }

+  public sendErrorEvent(
+    error: { message: string; stack?: string },
+    additionalData?: any,


I think commandName can be sent as part of additionalData.

Also, is it work deleting the sendError error method and update the code to use the new sendErrorEvent ?

You mean sendError in the core module? It has quite a few references, but not a ridiculous amount. I will create separate work for it. I'll modify additionalData though 👍

…into bp/refreshDefault

lcampos

…dx-vscode into bp/refreshDefault

brpowell added 2 commits April 5, 2019 13:23

sobject refresh default on and refactor to apex

807f5da

trailing comma

78e5198

brpowell requested review from ntotten, lcampos, allileong, JimSuplizio and AnanyaJha April 5, 2019 20:48

brpowell added 2 commits April 5, 2019 14:00

Merge branch 'develop' of github.com:forcedotcom/salesforcedx-vscode …

6a57e44

…into bp/refreshDefault

another trailing comma

e3b7b78

ruthemmanuelle reviewed Apr 5, 2019

View reviewed changes

lcampos reviewed Apr 8, 2019

View reviewed changes

packages/salesforcedx-vscode-core/src/index.ts Outdated Show resolved Hide resolved

lcampos reviewed Apr 8, 2019

View reviewed changes

packages/salesforcedx-vscode-apex/package.json Show resolved Hide resolved

improved telemetry for refresh and default off

11b9cdd

brpowell commented Apr 9, 2019

View reviewed changes

lcampos reviewed Apr 10, 2019

View reviewed changes

packages/salesforcedx-vscode-apex/src/index.ts Outdated Show resolved Hide resolved

lcampos reviewed Apr 10, 2019

View reviewed changes

Bryan added 2 commits April 10, 2019 08:36

review feedback

4278aad

Merge branch 'develop' of github.com:forcedotcom/salesforcedx-vscode …

be8e3c5

…into bp/refreshDefault

brpowell changed the title ~~sObject Refresh on extension activation is now on by default~~ Move sObject Refresh command to apex module and improve telemetry Apr 10, 2019

Bryan and others added 2 commits April 10, 2019 09:04

remove source from logname

8dc5bc4

test to verify telemetry data

faf4083

merge

25dfaf5

lcampos approved these changes Apr 10, 2019

View reviewed changes

Bryan added 2 commits April 11, 2019 11:15

merge develop

dd80107

Merge branch 'bp/refreshDefault' of github.com:forcedotcom/salesforce…

00a53fd

…dx-vscode into bp/refreshDefault

brpowell merged commit 3102ef9 into develop Apr 11, 2019

brpowell deleted the bp/refreshDefault branch April 11, 2019 19:43

This was referenced Jan 22, 2022

[Snyk] Fix for 4 vulnerabilities Sarrac3873/salesforcedx-vscode#28

Open

[Snyk] Fix for 20 vulnerabilities Sarrac3873/salesforcedx-vscode#31

Open

This was referenced Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#147

Open

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#148

Open

This was referenced Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#159

Open

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#160

Open

Sarrac3873 mentioned this pull request Jun 22, 2023

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#209

Open

This was referenced Nov 28, 2023

[Snyk] Fix for 9 vulnerabilities Sarrac3873/salesforcedx-vscode#248

Open

[Snyk] Fix for 5 vulnerabilities Sarrac3873/salesforcedx-vscode#249

Open

This was referenced May 13, 2024

[Snyk] Fix for 2 vulnerabilities Sarrac3873/salesforcedx-vscode#324

Open

[Snyk] Fix for 2 vulnerabilities Sarrac3873/salesforcedx-vscode#326

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move sObject Refresh command to apex module and improve telemetry #1236

Move sObject Refresh command to apex module and improve telemetry #1236

brpowell commented Apr 5, 2019 •

edited

Loading

codecov bot commented Apr 5, 2019 •

edited

Loading

ruthemmanuelle Apr 5, 2019

praksb commented Apr 8, 2019

brpowell commented Apr 8, 2019

lcampos commented Apr 8, 2019

praksb commented Apr 8, 2019

brpowell Apr 9, 2019

brpowell Apr 9, 2019

brpowell Apr 9, 2019

brpowell Apr 9, 2019

lcampos Apr 10, 2019

lcampos Apr 10, 2019

brpowell Apr 10, 2019

lcampos left a comment

		@@ -24,6 +24,21 @@ export interface CancellationToken {
		isCancellationRequested: boolean;

Move sObject Refresh command to apex module and improve telemetry #1236

Move sObject Refresh command to apex module and improve telemetry #1236

Conversation

brpowell commented Apr 5, 2019 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

codecov bot commented Apr 5, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

praksb commented Apr 8, 2019

brpowell commented Apr 8, 2019

lcampos commented Apr 8, 2019

praksb commented Apr 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcampos left a comment

Choose a reason for hiding this comment

brpowell commented Apr 5, 2019 •

edited

Loading

codecov bot commented Apr 5, 2019 •

edited

Loading